Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add various UI stuff #1948

Merged
merged 9 commits into from
Aug 18, 2024
Merged

Add various UI stuff #1948

merged 9 commits into from
Aug 18, 2024

Conversation

Masterjun3
Copy link
Collaborator

"Biggest" part of this PR is adding the submission topic vote count that we had on the old site 75% (9/3/2) (9 Yes, 3 Meh, 2 No).
I made a VoteCounts class for this. But I didn't know where to put it because it's both Forum Topic stuff and also Submission stuff. So I put it in Common. This can be moved wherever anyway.

So basically the PR adds this to the submission list,
image
and also to the forum topics.
image


The entire username now leads to the "last post". No need for the username to lead to the user profile. This can be done in a second click because you're about to go to the user's post which has the profile link there.
image


Claim now has an icon.

image


Fixed some bugs on the Wiki ViewSource page. On current revisions it was missing the number. And it always said "current" even on non-current revisions.

image


Added source links to revisions on Wiki PageHistory page, and also changed the coloring, because in dark mode this was not good:

image


Changed footer to button links, because those fit more with our other buttons:

image

@Masterjun3
Copy link
Collaborator Author

Masterjun3 commented Aug 18, 2024

Questions were raised about the performance of the added submission topic vote counts query.

The query on Subs-List with all statuses and the default 100 page size takes on average:

  • 0.100s before this PR.
  • 0.110s after this PR.

A bigger difference shows when maximizing the pagesize, loading all my ~7000 submissions at once. There the query takes on average:

  • 0.250s before this PR.
  • 0.700s after this PR.

Btw, it seems my tests in pgadmin slows down the queries by a factor of around 30-40. These queries in the EF Core log take all around 0.010s - 0.030s.

@adelikat adelikat merged commit 59225ec into main Aug 18, 2024
1 check passed
@adelikat adelikat deleted the minor-ui-adjustments-2 branch August 18, 2024 17:13
Copy link
Contributor

@RetroEdit RetroEdit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed footer to button links, because those fit more with our other buttons:

To me, the links are cleaner than buttons. It semantically and visually distinguishes overall site info from the Referrers, which is specific to your current page.

@Masterjun3
Copy link
Collaborator Author

The visual distinction could be done in a different way, e.g. by changing the location. This was previously discussed already.
In the end, at least we agreed that all buttons fits to the rest of our page in general.

A different position was already done in #1590 but eventually reverted. A discussion can be held about this. But in any case, this PR only converts the links into buttons.

@RetroEdit
Copy link
Contributor

I guess if this PR went through review, people think all buttons is a better stylistic fit.

Still, I would personally say to me buttons imply an amount of functionality and "action" in a web context. For example, with the wiki buttons, you're doing various wiki operations like Edit and Move that directly affect the page, and other buttons like Page History, Latest diff, and List referrers you're requesting information that can change based on wiki edits.

With the overall site info, it's the site version that links to a commit externally hosted on GitHub, Terms which is the static site rules, and API, which also can technically change, but it hosted externally.

Or to rephrase/reformulate my opinion, the other wiki buttons inherit the "interactive action" component that Edit and Move have, but the overall site info links are just that: links to places with information.

@vadosnaprimer
Copy link
Collaborator

We've been using buttons for internal links since day one of new site. The only distinction between action buttons and link buttons is color. Which makes me think - maybe making referrer link secondary color is enough?

@Masterjun3
Copy link
Collaborator Author

So basically there are two reasons why I replaced the links with buttons.

  1. It looks better visually because there was already the "List referrers" button, and that one couldn't be changed to a link because on wiki pages that button is next to other buttons.
  2. On mobile, in a cluster of links, buttons give a better visual clarity that it can be clicked, and also makes it easier to tap in general.

The "List referrers" button not fitting with the other buttons is a valid argument. But @RetroEdit I would rather like you make a separate issue ticket for this where stuff can be discussed, instead of discussing in a merged PR. You can link to this discussion in the new issue though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants